Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring SolidStateDetectors.jl extension #67

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

bene73
Copy link
Contributor

@bene73 bene73 commented Sep 18, 2024

In this new implementation of the SolidStateDetectors.jl extension, a config dictionary based on the detector metadata is created first and is afterwards used to create a SolidStateDetectors.SolidStateDetector or a SolidStateDetectors.Simulation object.

Previously it was only possible to create a SolidStateDetectors.SolidStateDetector object directly, which had no config dictionary. Thus, it was not possible to correctly save simulations in the HDF5 file format, which is now possible.

Except for the config dictionary, the resulting objects are the same as before.


The usage of the extension is the same as before, however now it is also possible to create a SolidStateDetectors.Simulation object:

using LegendDataManagement, SolidStateDetectors
l200 = LegendData(:l200)
sim = SolidStateDetectors.Simulation(l200, :V00050A)
det = SolidStateDetectors.SolidStateDetector(l200, :V00050A)

@bene73 bene73 changed the base branch from main to dev September 18, 2024 11:10
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 73.82550% with 39 lines in your changes missing coverage. Please review.

Project coverage is 33.97%. Comparing base (f631749) to head (abb3159).
Report is 8 commits behind head on dev.

Files with missing lines Patch % Lines
ext/LegendDataManagementSolidStateDetectorsExt.jl 73.82% 39 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev      #67      +/-   ##
==========================================
- Coverage   34.25%   33.97%   -0.28%     
==========================================
  Files          28       28              
  Lines        1854     1863       +9     
==========================================
- Hits          635      633       -2     
- Misses       1219     1230      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@fhagemann fhagemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good to go now.

end


function create_SSD_config_dict_from_LEGEND_metadata(meta::PropDict, xtal_meta::X; dicttype = Dict{String,Any}) where {X <: Union{PropDict, LegendDataManagement.NoSuchPropsDBEntry}}
Copy link
Contributor

@fhagemann fhagemann Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the dependency on OrderedCollections and introduced the keyword dicttype which (by default) is set to Dict{String, Any}.
We can reintroduce the dependency on OrderedCollections in a follow-up PR, but it is not needed in this refactoring PR.

Comment on lines -84 to +23
sim.detector = SolidStateDetector(sim.detector, contact_id = 1, contact_potential = 0) # Set potential to 0V to avoid long simulation times
sim.detector = SolidStateDetector(sim.detector, contact_id = 2, contact_potential = 0) # (we just need the PointTypes for the active volume)
calculate_electric_potential!(sim, max_tick_distance = 0.1u"mm", refinement_limits = missing, verbose = false)
SolidStateDetectors.apply_initial_state!(sim, ElectricPotential, Grid(sim, max_tick_distance = 0.1u"mm"))
Copy link
Contributor

@fhagemann fhagemann Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this prevents the tests from failing on Julia 1.11

Comment on lines +28 to +35
# The creation via config files allows to save Simulations to files using LegendHDF5IO
lh5name = "$(detname).lh5"
isfile(lh5name) && rm(lh5name)
@test_nowarn ssd_write(lh5name, sim)
@test isfile(lh5name)
@test sim == ssd_read(lh5name, Simulation)
@test_nowarn rm(lh5name)
@test !isfile(lh5name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests would previously fail:
Now that Simulation has a config_dict, the sim can actually be written and read using LegendHDF5IO.

@theHenks
Copy link
Collaborator

Great work, thanks @bene73 and @fhagemann . Could you maybe add the details about the Simulation object in the documentation for the extension. That would be awesome!

@fhagemann
Copy link
Contributor

Done!

@fhagemann fhagemann merged commit 7f2a917 into legend-exp:dev Sep 19, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants